Add vitest and unit tests for existing functionality #123
Conversation
jamaljsr
force-pushed
the
add-unit-tests
branch
2 times, most recently
from
eedb701 to
2366bd4
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jamaljsr
force-pushed
the
add-unit-tests
branch
from
2366bd4 to
f2e8b5a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jamaljsr
force-pushed
the
add-unit-tests
branch
from
f2e8b5a to
04a3506
Compare
jamaljsr
force-pushed
the
add-unit-tests
branch
from
04a3506 to
e785952
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive unit test coverage to the Lightning Node Connect (LNC) web library using Vitest as the testing framework. The purpose is to establish test coverage for existing functionality to prevent breaking changes during future improvements.
- Replaces Jest/Mocha with Vitest for faster test execution
- Adds extensive test coverage across all core modules with 129 passing tests
- Integrates GitHub Actions workflow for automated CI testing
Reviewed Changes
Copilot reviewed 19 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Configuration for Vitest test runner with coverage settings |
| package.json | Updates test scripts and dependencies to use Vitest |
| test/setup.ts | Global test setup with mocks for browser APIs and WebAssembly |
| test/mocks/* | Mock implementations for localStorage, WebAssembly, and test utilities |
| test/utils/* | Helper utilities and factories for test data and mock setup |
| lib/**/*.test.ts | Comprehensive unit tests for all core modules |
| lib/lnc.ts | Minor refactoring to use typed global references |
| lib/util/log.ts | Added getter for log level property |
| tslint.json | Removed unused variable rule |
| .prettierignore | Added coverage directory exclusion |
| .github/workflows/test.yml | GitHub Actions workflow for running tests in CI |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const mockLnc = { | ||
| request: vi.fn(), | ||
| subscribe: vi.fn() | ||
| } as unknown as Mocked<LNC>; |
There was a problem hiding this comment.
do we need the as unknown as Mocked<LNC>? Could we instead have it be a partial?
There was a problem hiding this comment.
If we remove the unknown there will be a TS error.
jamaljsr
force-pushed
the
add-unit-tests
branch
from
e785952 to
2ddcb0c
Compare
There was a problem hiding this comment.
@jbrill Thanks for the review. I've updated addressed all of your feedback.
Nit: we could have the coverage summary a little more compact for readability if we wanted:
--reporter=text-summary
text-summary isn't an available reporter for vitest. I don't think the current output is too verbose.
--reporter <name> Specify reporters (default, basic, blob, verbose, dot, json, tap, tap-flat, junit, hanging-process, github-actions)
|
LGTM! Thank you for the iterations on this 🙏 |
jamaljsr
force-pushed
the
add-unit-tests
branch
from
2ddcb0c to
35fda72
Compare
jamaljsr
force-pushed
the
add-unit-tests
branch
from
35fda72 to
040b2d3
Compare
jamaljsr
force-pushed
the
add-unit-tests
branch
from
040b2d3 to
e5aa7ea
Compare
In preparation for some upcoming improvements to this library, I have added many unit tests to ensure there are no breaking changes to the existing functionality going forward. I chose to use the Vitest testing framework over Jest because it is substantially faster. I have also added a Github workflow job to run the tests in CI.
Steps to Test
yarnto update yournode_modulesyarn test:coverageto run the testsExample
$ yarn test:coverage yarn run v1.22.17 $ vitest run --coverage RUN v3.2.4 /Users/jamal/dev/lnc-web Coverage enabled with v8 ✓ lib/util/log.test.ts (20 tests) 6ms ✓ lib/api/createRpc.test.ts (9 tests) 4ms ✓ lib/util/credentialStore.test.ts (27 tests) 15ms ✓ lib/util/encryption.test.ts (19 tests) 17ms ✓ lib/index.test.ts (2 tests) 59ms ✓ lib/lnc.test.ts (52 tests) 98ms Test Files 6 passed (6) Tests 129 passed (129) Start at 08:02:38 Duration 468ms (transform 229ms, setup 122ms, collect 397ms, tests 198ms, environment 1ms, prepare 258ms) % Coverage report from v8 ---------------------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s ---------------------|---------|----------|---------|---------|------------------- All files | 99.37 | 96.26 | 98.14 | 99.37 | lib | 98.74 | 94.91 | 95 | 98.74 | index.ts | 70 | 100 | 0 | 70 | 9-11 lnc.ts | 100 | 94.91 | 100 | 100 | 62,203 lib/api | 100 | 100 | 100 | 100 | createRpc.ts | 100 | 100 | 100 | 100 | lib/types | 0 | 0 | 0 | 0 | lnc.ts | 0 | 0 | 0 | 0 | lib/util | 100 | 97.01 | 100 | 100 | credentialStore.ts | 100 | 95.34 | 100 | 100 | 221,232 encryption.ts | 100 | 100 | 100 | 100 | log.ts | 100 | 100 | 100 | 100 | ---------------------|---------|----------|---------|---------|------------------- ✨ Done in 0.94s.